-
Notifications
You must be signed in to change notification settings - Fork 469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Utility class to detect Typescript functions that can possibly be used as Lambda Function Handlers. #142
Conversation
…d as Lambda Function Handlers.
* @description Looks for functions that appear to be valid Lambda Function Handlers. | ||
* @returns A collection of information for each detected candidate. | ||
*/ | ||
public async findCandidateLambdaHandlers(): Promise<LambdaHandlerCandidate[]> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What's the performance on very large (i.e. 10,000+ lines, many functions) files?
- Will this be called from the UI thread?
- Is it feasible to only parse code that's currently in the user's viewport (i.e., not scrolled out of sight)?
- Is it feasible to return a
Promise<LambdaHandlerCandidate | undefined>[]
(or aPromise<Promise<LambdaHandlerCandidate | undefined>[]>
) instead? It would be awesome if we could update the view for each function independently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I've seen of CodeLens, extensions are given the file as a whole, not for a viewport.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've put the performance investigation into #145
Codecov Report
@@ Coverage Diff @@
## develop #142 +/- ##
===========================================
+ Coverage 39.92% 44.66% +4.74%
===========================================
Files 48 49 +1
Lines 1250 1359 +109
Branches 155 189 +34
===========================================
+ Hits 499 607 +108
- Misses 751 752 +1
Continue to review full report at Codecov.
|
* @param targetFunctionNames Names of functions of interest | ||
*/ | ||
private static isValidFunctionAssignment(expression: ts.Expression): boolean { | ||
if (ts.isFunctionLike(expression)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, does isFunctionLike
support cases like = function() { /* ... */ }
? The whole API is undocumented, which makes it hard to review 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved pending validation of performance on large files.
Types of changes
Description
This code provides a way of locating Typescript functions that can be used as Lambda Function Handlers. This uses the Typescript engine instead of Symbol Information that can be surfaced by VS Code, since VS Code doesn't seem to provide parameter information for functions and methods.
Motivation and Context
Related Issue(s)
#140 - while this is not surfaced presently, it will be used in #135 and #136 when CodeLens looks to determine which functions can be invoked/debugged.
Testing
Screenshots (if appropriate)
Checklist
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.